test(cosim): pin the CpuBootTrace wire layout to a literal - #437
Merged
Conversation
The testbench in the sibling repository now writes this format, so cpu_boot_trace_diff -- already written, already tested, and already used against a third-party reference emulator -- reads a device-under-test's output with no modification at all. Demonstrated rather than asserted: a C++-written 256-record trace loads and reports "All 256 aligned records match", and a corrupted register is located at cyc=307 PC=$C064 A=$64 against A=$9B. The pin exists so the two implementations are anchored to the SAME hardcoded bytes rather than to each other. Anchoring them to each other has a specific failure: they can agree on something wrong, and neither test notices. Anchoring both to a literal means a drift on either side reddens that side's own test, at the moment the drift happens -- rather than at co-simulation time, where a format difference is indistinguishable from a DUT defect and would be debugged as one. It also pins the constants the C++ header duplicates: the magic, the schema version, HEADER_SIZE and RECORD_SIZE. Those are four more values that live in two repositories with nothing mechanical connecting them. scanline = -1 in the pinned record is deliberate, not an arbitrary value. The pre-render line is negative, and a writer that clamped or saturated instead of writing two's complement would pass every test that only ever used a positive scanline -- the C++ side carries a matching check that a POSITIVE scanline encodes differently, so neither case is proving something both branches would satisfy. Demonstrated to fail: changing the two scanline bytes in the expectation reddens the test. Gates. Test-only in the excluded crate; the emulation core is untouched, so AccuracyCoin 141/141 and nestest 0-diff hold by construction. fmt, clippy -D warnings and rustdoc -D warnings on the excluded crate, markdownlint on the changed document. 4 excluded-crate suites / 43 passed / 0 failed (was 42)
|
Warning Your free Security trial is over. An organization admin can activate billing to continue. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Antigravity review (Gemini via Ultra)This PR pins the Blocking issuesNone found. SuggestionsNone. NitpicksNone. Automated first-pass review by |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The sibling testbench now writes the oracle's
CpuBootTraceformat, socpu_boot_trace_diff— already written, already tested, and already used against a third-party reference emulator — reads a device-under-test's output with no modification at all.Demonstrated rather than asserted, on a C++-written 256-record trace:
That is the plan's claim — a ~120-line C++ record writer buys the existing diff CLIs unmodified — met. The alternative it avoids is worth naming: a comparison written specifically for the DUT would be new, unexercised code on the one axis where new code is least welcome, namely the thing that decides whether the DUT is correct.
Why a literal, and not each other
Anchoring the two implementations to each other has a specific failure: they can agree on something wrong, and neither test notices. Anchoring both to the same hardcoded bytes means a drift on either side reddens that side's own test, at the moment it happens — rather than at co-simulation time, where a format difference is indistinguishable from a DUT defect and would be debugged as one.
It also pins the four constants the C++ header duplicates: the magic, the schema version,
HEADER_SIZE,RECORD_SIZE. Those live in two repositories with nothing mechanical connecting them.scanline = -1is deliberateThe pre-render line is negative, and a writer that clamped or saturated instead of writing two's complement would pass every test that only ever used a positive scanline. The C++ side carries a matching check that a positive scanline encodes differently, so neither case is proving something both branches would satisfy.
Demonstrated to fail: changing the two scanline bytes in the expectation reddens the test.
Gates
Test-only in the excluded crate; the emulation core is untouched, so AccuracyCoin 141/141 and nestest 0-diff hold by construction.
fmt · clippy
-D warnings· rustdoc-D warningson the excluded crate · markdownlint.4 excluded-crate suites / 43 passed (was 42).